fix(sandbox): resolve symlinks before path containment check in sandboxed fs.readFile - #13

• Resolves symlinks before path containment check in sandboxed fs.readFile
• Prevents CWE-22 path traversal via symlink bypass in security-critical sandbox
• Uses fs.realpath() to canonicalize paths before allowlist validation
• Passes resolved path to readFile() to ensure correct file access

Add symlink resolution to fs.readFile security check

• Added realpath import from node:fs/promises to resolve symbolic links
• Modified fs.readFile handler to await realpath() on resolved path before containment check
• Updated readFile call to use the fully resolved path instead of original filePath
• Prevents symlink-based path traversal attacks within allowed root directories

src/cognition/emergent/SandboxedToolForge.ts

Description

fs.readFile calls realpath() on attacker-controlled paths before verifying lexical containment,
so out-of-root probes can throw ENOENT/EACCES that bubble to the caller and disclose host
filesystem existence/permission info. This is a security regression vs. consistently returning the
sandbox "blocked" error for out-of-root paths.

Code

src/cognition/emergent/SandboxedToolForge.ts[R477-480]

Evidence

The sandboxed fs.readFile resolves the path via realpath() before determining whether it is
allowed, and errors thrown from sandbox execution are returned to the caller as part of the
execution error string, enabling out-of-root probing via differing error messages.

src/cognition/emergent/SandboxedToolForge.ts[474-487]
src/cognition/emergent/SandboxedToolForge.ts[398-410]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`realpath(path.resolve(filePath))` is executed before any allowlist/root containment decision. If the requested path is outside `fsReadRoots`, `realpath()` can throw `ENOENT`/`EACCES` and the error is surfaced back to the sandbox caller, leaking host filesystem details (existence/permissions) that the sandbox is intended to hide.
### Issue Context
You need symlink resolution to prevent CWE-22, but the implementation should avoid touching the filesystem for paths that are clearly outside the allowed roots.
### Fix Focus Areas
- src/cognition/emergent/SandboxedToolForge.ts[474-493]
### Suggested approach
1. Compute `const candidate = path.resolve(filePath)`.
2. Perform a **lexical** containment check against `this.fsReadRoots` using `candidate`.
- If not lexically within any root, throw the existing `fs.readFile blocked: ... outside the allowed roots` error **without calling `realpath()`**.
3. Only after lexical acceptance, compute `const resolvedPath = await realpath(candidate)`.
4. Perform a **physical** containment check (see separate finding about canonicalizing roots) and then `readFile(resolvedPath)`.
5. Optionally normalize/catch any `realpath` failures for lexically-allowed paths to a consistent sandbox error format if you do not want to leak details even within roots.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The candidate path is now canonicalized with realpath(), but fsReadRoots are still only
path.resolve()'d, so containment can incorrectly fail when an allowed root includes symlink
components (real path won’t start with the lexical root). This can block legitimate reads for
deployments that configure fsReadRoots using symlinked directories.

Code

src/cognition/emergent/SandboxedToolForge.ts[R477-480]

Evidence

fsReadRoots are computed with path.resolve() in the constructor, while fs.readFile now uses
realpath() for the candidate path; comparing these two different canonical forms breaks prefix
checks when roots contain symlinks.

src/cognition/emergent/SandboxedToolForge.ts[172-178]
src/cognition/emergent/SandboxedToolForge.ts[474-480]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`resolvedPath` is a physical path (via `realpath()`), but `this.fsReadRoots` are stored as lexical `path.resolve()` results. If a configured root contains symlinks, the physical path prefix check can fail and deny access even though the file is under the configured root directory.
### Issue Context
This arises due to mixing lexical canonicalization (roots) with physical canonicalization (candidate). For correctness, both sides of the containment check should use the same canonicalization strategy.
### Fix Focus Areas
- src/cognition/emergent/SandboxedToolForge.ts[172-178]
- src/cognition/emergent/SandboxedToolForge.ts[474-493]
### Suggested approach
- Canonicalize `fsReadRoots` using `realpath()` as well.
- Since the constructor cannot `await`, consider:
 - Adding a private cached promise (e.g., `private fsReadRootsReal: Promise<string[]>`) initialized lazily on first `fs.readFile` call, or
 - Switching root canonicalization to a sync call (e.g., `fs.realpathSync`) if acceptable.
- Use the canonicalized root list for the physical containment check against `resolvedPath`.
- Keep lexical check (if you implement it per Finding 1) using the lexical roots, then do the physical check using realpathed roots.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗